perf(parquet): use Cursor in ZSTDCodec to avoid Vec alloc and copy - #10345
Conversation
47a9e23 to
02523a9
Compare
|
run benchmark compression |
This comment was marked as duplicate.
This comment was marked as duplicate.
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
run benchmark compression |
This comment was marked as duplicate.
This comment was marked as duplicate.
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Jefffrey
left a comment
There was a problem hiding this comment.
None, although there is an implementation detail to consider; Cursor cannot be limited when writing, so this change technically allows compress/decompress to write more data than before, based on excess Vec capacity.
Vec::with_capacityis not required to allocate exactly, so this should be just fine.
could you elaborate on this a bit?
|
And to explain the |
|
run benchmark compression |
This comment was marked as duplicate.
This comment was marked as duplicate.
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
thanks @MassivePizza |
|
Maybe not a problem, as described in #10448 :
|
Which issue does this PR close?
N/A
Rationale for this change
Remove needless allocs and copies.
What changes are included in this PR?
Inline
zstd::bulk::methods that returned a temporary Vec.Reserve space and write to output_buf directly using a Cursor instead.
Are these changes tested?
Should be covered by existing tests.
Are there any user-facing changes?
None, although there is an implementation detail to consider; Cursor cannot be limited when writing, so this change technically allows compress/decompress to write more data than before, based on excess Vec capacity.
Vec::with_capacityis not required to allocate exactly, so this should be just fine.